rustdoc: properly support macros with multiple kinds#152449
Conversation
|
Some changes occurred in HTML/CSS/JS. |
This comment has been minimized.
This comment has been minimized.
20f13be to
4b35c49
Compare
This comment has been minimized.
This comment has been minimized.
4b35c49 to
4416897
Compare
|
Oh and also cc @lolbinarycat as you reviewed the original PR too. :) |
This comment has been minimized.
This comment has been minimized.
4416897 to
be7815f
Compare
This comment has been minimized.
This comment has been minimized.
be7815f to
3e05767
Compare
This comment has been minimized.
This comment has been minimized.
|
Fixed merge conflicts. |
This comment has been minimized.
This comment has been minimized.
|
blocking this on #154902, since that relies on |
3e05767 to
ce98d4e
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ce98d4e to
5ada01c
Compare
5ada01c to
26f32d9
Compare
This comment has been minimized.
This comment has been minimized.
eef2a58 to
cbe154c
Compare
Rename `ItemType::BangMacro*` into `ItemType::DeclMacro*` Rename `isBangMacro` field into `forceMacroHref`
Add more typescript type annotations Improve functions naming
718a495 to
794301a
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Needed to rebase because some changes were incompatible. |
| /// for `macro_rules!` items which contain an attr/derive kind. | ||
| pub(crate) fn types(&self) -> impl Iterator<Item = ItemType> { | ||
| if let ItemKind::MacroItem(_, macro_kinds) = self.kind { | ||
| Either::Right(macro_kinds.iter().map(|kind| match kind { |
There was a problem hiding this comment.
Hey using macro_kinds.iter() here means the order of the iterator depends on bitflags iteration order, which I don't love.
There was a problem hiding this comment.
As far as I know, the order never matters since it's the same item with potentially different types.
|
|
||
| #![feature(macro_attr)] | ||
| #![feature(macro_derive)] | ||
|
|
There was a problem hiding this comment.
I think we're missing a testcase:
#[macro_export]
macro_rules! macro5 {
attr() () => {};
derive() () => {};
}There was a problem hiding this comment.
Indeed, adding it.
|
Added the missing test case, also improved the search code clarity a bit. |
There was a problem hiding this comment.
Alright, I think it's ready, lets try this.
@bors r=lolbinarycat,mejrs
| let tag = if my_section == super::ItemSection::Reexports { | ||
| REEXPORTS_TABLE_OPEN | ||
| } else { | ||
| ITEM_TABLE_OPEN | ||
| }; | ||
| write!( | ||
| w, | ||
| "{}", | ||
| write_section_heading(my_section.name(), &cx.derive_id(my_section.id()), None, tag) | ||
| )?; |
There was a problem hiding this comment.
This refactor made me realize there's no snapshot test for sidebar generation. I think I'll submit a PR for that.
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing be326a9 (parent) -> d3cd040 (this PR) Test differencesShow 16 test diffsStage 1
Stage 2
Additionally, 4 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard d3cd04068e406dd961c2fd666c049c3e38930e0a --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (d3cd040): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Our benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 2.8%, secondary 2.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 3.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 511.489s -> 510.352s (-0.22%) |
|
The impact is expected since we added some loops for iterating types for an item (even though 99.9% of the time, there is only one type). |
|
A job failed! Check out the build log: (web) (plain enhanced) (plain) Click to see the possible cause of the failure (guessed by this bot) |
View all comments
Since it seems like I can't reopen #145458, opening this one. Although, it's the same PR minus the last new commit to handle a comment that was left unresolved in the original PR. All relevant details are still in the original PR though.
It's an alternative (and likely a take-over) of #148005 since lang-team rejected the idea to add documentation on macro branches, making the multiple files approach less suitable.
This implements #145153 in rustdoc. This PR voluntarily doesn't touch anything related to intra-doc links, I'll send a follow-up if needed.
So now about the implementation itself: this is a weird case where a macro can be different things at once but still only gets one file generated. I saw two ways to implement this:
ItemKind::Macrodifferently and iter through itsMacroKindsvalues.ItemKindenum, which means that when we encounter them in rendering, we need to ignore them. It also makes theItemKindenum bigger (and also needs more code to be handled). Another downside is that it needs to be handled in the JSON output.Now there was an interesting improvement that came with this PR in the
html::render::print_item::item_modulefunction: I simplified its implementation and split the different parts in aHashMapwhere the key is the item type. So then, we can just iterate through the keys and open/close the section at each iteration instead of keeping anOption<section>around.From RFCs:
macro_rules!) attribute macros (RFC 3697) #144579macro_rules!) derive macros (RFC 3698) #145208derive:
attr:
both:
r? @notriddle